Conversation
Summary of ChangesHello @roderik, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the network bootstrapper by enabling the publication of a subgraph IPFS hash as a Kubernetes ConfigMap or a local file. This feature allows users to easily manage and distribute the hash of their deployed subgraphs, ensuring consistency across environments. The change introduces a new CLI option and environment variable for specifying the hash file, along with validation to ensure the hash's integrity. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
To view in Slack, search for: 1758440325.371649 |
There was a problem hiding this comment.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/cli/commands/bootstrap/bootstrap.command.ts:407` </location>
<code_context>
? await deps.loadAbis(trimmedAbiDirectory)
: [];
+ const envSubgraphHashFile = Bun.env.SUBGRAPH_HASH_FILE?.trim();
+ const providedSubgraphHashFile =
+ subgraphHashFileOption === undefined
</code_context>
<issue_to_address>
**suggestion:** Subgraph hash file path is trimmed but not normalized for possible surrounding quotes.
Please apply `stripSurroundingQuotes` to the subgraph hash file path to ensure consistent normalization and prevent issues when quotes are present.
Suggested implementation:
```typescript
const envSubgraphHashFile = stripSurroundingQuotes(Bun.env.SUBGRAPH_HASH_FILE?.trim());
const providedSubgraphHashFile =
subgraphHashFileOption === undefined
? undefined
: stripSurroundingQuotes(subgraphHashFileOption.trim());
let subgraphHashPath: string | undefined;
if (providedSubgraphHashFile && providedSubgraphHashFile.length > 0) {
subgraphHashPath = providedSubgraphHashFile;
} else if (envSubgraphHashFile && envSubgraphHashFile.length > 0) {
subgraphHashPath = envSubgraphHashFile;
}
```
Make sure that `stripSurroundingQuotes` is imported or available in this file. If it's not already imported, add:
```ts
import { stripSurroundingQuotes } from 'path/to/stripSurroundingQuotes';
```
at the top of the file, adjusting the import path as needed.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Code Review
This pull request effectively adds support for publishing a subgraph IPFS hash as a ConfigMap, mirroring the handling of other network artifacts. The changes include a new CLI option, environment variable support, a validation helper for the hash, and integration into both file and Kubernetes outputs. The accompanying tests are thorough, covering the new logic and ensuring existing functionality remains intact. I've provided a couple of suggestions to simplify some of the new logic, which will enhance readability and maintainability.
| if (originalEnv === undefined) { | ||
| Bun.env.SUBGRAPH_HASH_FILE = undefined; | ||
| } else { | ||
| Bun.env.SUBGRAPH_HASH_FILE = originalEnv; | ||
| } |
There was a problem hiding this comment.
The finally block for restoring the SUBGRAPH_HASH_FILE environment variable can be simplified. The if/else statement is not necessary, as assigning originalEnv (which can be a string or undefined) directly to Bun.env.SUBGRAPH_HASH_FILE achieves the same result more concisely.
Bun.env.SUBGRAPH_HASH_FILE = originalEnv;| const envSubgraphHashFile = Bun.env.SUBGRAPH_HASH_FILE?.trim(); | ||
| const providedSubgraphHashFile = | ||
| subgraphHashFileOption === undefined | ||
| ? undefined | ||
| : subgraphHashFileOption.trim(); | ||
| let subgraphHashPath: string | undefined; | ||
| if (providedSubgraphHashFile && providedSubgraphHashFile.length > 0) { | ||
| subgraphHashPath = providedSubgraphHashFile; | ||
| } else if (envSubgraphHashFile && envSubgraphHashFile.length > 0) { | ||
| subgraphHashPath = envSubgraphHashFile; | ||
| } |
There was a problem hiding this comment.
The logic for determining subgraphHashPath by prioritizing the CLI option over the environment variable is more verbose than necessary. This can be simplified into a single line using the logical OR (||) operator, which improves readability and conciseness. The subgraphHashFileOption is already a trimmed string or undefined from the command's action handler, so no further processing is needed on it.
| const envSubgraphHashFile = Bun.env.SUBGRAPH_HASH_FILE?.trim(); | |
| const providedSubgraphHashFile = | |
| subgraphHashFileOption === undefined | |
| ? undefined | |
| : subgraphHashFileOption.trim(); | |
| let subgraphHashPath: string | undefined; | |
| if (providedSubgraphHashFile && providedSubgraphHashFile.length > 0) { | |
| subgraphHashPath = providedSubgraphHashFile; | |
| } else if (envSubgraphHashFile && envSubgraphHashFile.length > 0) { | |
| subgraphHashPath = envSubgraphHashFile; | |
| } | |
| const subgraphHashPath = | |
| subgraphHashFileOption || Bun.env.SUBGRAPH_HASH_FILE?.trim(); |
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_68cf2d48dc44833287e31391dee5278e
Summary by Sourcery
Add support for loading and publishing a subgraph IPFS hash as a ConfigMap in the network bootstrap process.
New Features:
Build:
Documentation:
Tests: